Make ROSE containers more friendly #485

yaacov · 2023-09-11T13:25:17Z

TL;DR demo

# Start student A driver container
podman run --rm --network host -it quay.io/rose/rose-ml-driver:latest --port 8081

# Start student B driver container
podman run --rm --network host -it quay.io/rose/rose-go-driver:latest --port 8082

# Start server container, and connect to the students drivers
podman run --rm --network host -it quay.io/rose/rose-server:latest --drivers \
     http://127.0.0.1:8081 http://127.0.0.1:8082

# Open browser on local machine at: http://127.0.0.1:8880

Screencast.from.2023-09-11.16-09-37.webm

Ref:
Python machine learning driver: https://github.com/yaacov/rose-ml-driver
Go land driver: https://github.com/yaacov/rose-go-driver

What this POC try to solve:

a. We can not create a k8s Pod for each driver, and let them run forever.
When a driver starts it immediately try to connect to the server and blocks other drivers from running.
b. We use python RPC protocol to comunicate between the driver and the server, this makes it hard to write
drivers in compiled languages, making us relay on .pyc file to hide implementation code.
c. We use 2.7 oriented libraries: twisted, autobahn and pytest.

Fix a and b:
Replace the RPC calls with "vanilla" client server calls, the server can now connect to selected drivers
while the other drivers still running.
Fix c:
Replace the 2.7 oriented libraries with 3.x native libraries, websockets and aiohttp.

What is changing:

The admin needs to actively select the drivers to run the match instead of waiting for two students to connect.
All students must run there drivers before the match begin.

Running locally:

# Run the client using an example driver (default port 8081, use --port to change)
./rose-client --driver examples/random-driver.py

# On a different terminal (tmux session)
./rose-server

# Connect the server to the client
./rose-admin http://127.0.0.1:8880 set-drivers http://127.0.0.1:8081

# Start a match
./rose-admin http://127.0.0.1:8880 start

yaacov · 2023-09-11T14:29:48Z

@nirs can you take a look ?
@bennypowers hi, can you take a look at the js parts ?
@slaviered hi, this should not change the docs and classroom documentation, if it does, I will fix them.

bennypowers · 2023-09-11T15:06:57Z

rose/server/web/index.html

@@ -9,11 +9,20 @@

  <body>
    <div id="content">
+      <a href="settings.html" id="settings-link">Settings</a>


you might want to make this settings/ and rename settings.html to settings/index.html

moved to '/settings/index.html'
note that the python server we are using can't automatically show index.html files when the request is / it show 404

@bennypowers i currently put the web component in settings-form file in the same dir, is there a convention for how to name web components ? where to put tehm ?

Usually, name the module the same as the tag name

bennypowers · 2023-09-11T15:07:43Z

rose/server/web/index.html

+      <a href="settings.html" id="settings-link">Settings</a>
+      <a href="#" id="info-btn">Info</a>
+
+      <div id="info-panel" class="hidden">


class=hidden vs hidden vs use a pre-built custom-element with aria announce stuff ready to go

ARIA is a little out of scope here :-)
we may implement when we move to web components

While aria is not necessary, the experience must be available to screen reader users

bennypowers · 2023-09-11T15:08:31Z

rose/server/web/index.html

        <button id="stop"  disabled>Stop</button>
        <button id="music_ctl">Music</button>
+        <button id="reset">Reset</button>


Suggested change

<button id="reset">Reset</button>

<button id="reset" type="reset">Reset</button>

bennypowers · 2023-09-11T15:08:56Z

rose/server/web/settings.css

@@ -0,0 +1,84 @@
+body.dark-theme {
+    background-color: #2c3e50;


consider using red hat design tokens

https://ux.redhat.com/tokens

rose/server/web/settings.html

bennypowers · 2023-09-11T15:11:29Z

rose/server/web/settings.html

+        document.addEventListener('DOMContentLoaded', loadDrivers);
+    </script>
+</head>
+<body class="dark-theme">


use prefers-color-scheme

rose/server/web/settings.html

bennypowers · 2023-09-11T15:12:34Z

rose/server/web/settings.html

+            <input type="text" id="driver2" name="driver2" class="input-field">
+        </div>
+        <div class="form-group">
+            <button type="submit" class="submit-button">Send</button>


don't need the type here. or really the class

added a type to prevent form event trigger, can I avoid it ?

ahh, sorry, it now a regular button ... this one do need the type :-)

if you need to prevent default, do it in the custom element class, otherwise havea route on the http server that accepts the http formdata, for progressive enhancement

rose/server/web/settings.html

nirs · 2023-09-11T15:36:55Z

Cool demo!

But about 20x times too big to review. Can you break up to small easy to review commits?

In general the idea that the server connects to the drivers looks strange and harder to use.

you need to run the server after the clients
you need to know the clients address to run the server
you need to restart the server for every game
the server need to handle client disconnection - previously the client could reconnect

The current model when the clients connects to the server is better. I don't see why we need
to change it.

Regarding the problems:

a. We can not create a k8s Pod for each driver, and let them run forever.
When a driver starts it immediately try to connect to the server and blocks other drivers from running.

Why do you want to run the client forever? The client is something you run temporarily when testing or during a game.

b. We use python RPC protocol to comunicate between the driver and the server, this makes it hard to write drivers in compiled languages, making us relay on .pyc file to hide implementation code.

This is incorrect. We use simple line based JSON lines RPC protocol that is easy to write
any any languages and is not related to python in any way.

The protocol for for communicating with the server is not related to the the driver pyc file in any way. The dependency is the python client which can be replaced with a client written in any other language.

c. We use 2.7 oriented libraries: twisted, autobahn and pytest.

These libraries are not python 2.7 oriented, they just mature libraries that were popular
years before python 3 was started.

Twisted is awesome because it let you run multiple services clients in the same process. I don't think there is anything replacing it. For example, if you want to add an IRC clinet reporting game results to IRC channel, this is really easy in twisted.

autobahn is web socket library with twisted integartion. websoket is the best way to commmunicate with the game UI, allowing real time updates. Why replace it?

pytest is the best testing library for python (if not the best testing library for anything).
I'm not sure what is the issue with it.

Since we used twisted, it was very easy to add xmlrpc server for the admin tool, but we
should realy get rid of the xmlrpc code since simple http (already used by the UI) is
good enough for the admin tool, or maybe we can just drop the admin tool - since the
web UI was added, there is no need for the admin tool.

It will be more productive to open issues for any problem and discuss them.

yaacov · 2023-09-11T15:55:58Z

@nirs thanks

This PR is to make the Game more Openshift ready, when I run pods in Openshift, I want to deploy my pods, and let them run.

Why do you want to run the client forever? The client is something you run temporarily when testing or during a game.

"As an instructor I want to ask the students to deploy their drivers to the cluster, and then run matches using an already running server and drivers"

removing the xmlrpc connection between the driver and server makes it easy to run the game-engine (named "server") and the driver (named "client") as Pods that live as good citizens of the cluster.

you need to run the server after the clients

you don't need to re-run the server, the idea is that the server is running all the time.
you only need to choose the drivers for the next match, and you do it while the server is running.

you need to know the clients address to run the server

this is easy when running on a cluster:

oc get services -n students

you need to restart the server for every game

the server is good citizen of the cluster, it is deployed once and run forever,
the only caveat is that the game engine "server" has a state while running a game, so you can only have one "server" serving a set of games.

you can run several game-engines "servers" but they will need to have a URL per server and can't run using a load balancer, the drivers on the other hand are stateless and can run in deployments of more then one.

the server need to handle client disconnection - previously the client could reconnect

the game-engine (named "server" because it is currently called this way) is a client or the driver (named "client" because it is currently called this way) server, and does not care if the driver is connected or not, it just pool it, it can get an answer of miss it, nothing will happen if no answer is given except the the car will continue forward and the user will get a notice.

yaacov · 2023-09-11T16:05:11Z

This is incorrect. We use simple line based JSON lines RPC protocol that is easy to write
any any languages and is not related to python in any way.

The protocol for for communicating with the server is not related to the the driver pyc file in any way. The dependency is the python client which can be replaced with a client written in any other language.

@nirs ok, so it uses twisted protocol instead of xmlrpc ...
https://twisted.org/documents/8.1.0/api/twisted.protocols.basic.LineReceiver.html

not sure if it's that easy to implement in other languages, but I didn't try :-)
I still think that simple HTTP client server is more clean and clear, and easy to implement in "any language" (tm)

yaacov · 2023-09-11T16:06:04Z

Can you break up to small easy to review commits?

Yes, I will see how to make it more clear 👍

yaacov · 2023-09-11T16:19:50Z

Twisted vs asyncio

asyncio is a part of the standard library. no need to install it, it's a nice plus for using it.
asyncio: Uses async/await syntax, this is debatable, but I think that most users like it, this pattern is very common in other langauges too, so it's also a plus.
asyncio: works nice with aiohttp, while twisted works nice with autobahn ... so it's a tie here :-)

To sum things up, Twisted is awesome, but asyncio is integrated with python and is "the new cool thing"
for me it's 2 plus for asyncio and one tie :-)

yaacov · 2023-09-11T16:22:47Z

pytest vs unittest

unittest: Part of the Python standard library. You don't need to install anything extra to use it.
unittest: Doesn't support parameterized testing out of the box. You'd need to use additional tools or libraries

so it's one plus for unittest and one for pytest :-)
but the fact it's part of Python makes me think it's nicer to use.

nirs · 2023-09-11T16:40:28Z

Twisted vs asyncio

asyncio does not replace twisted - it is just the core of twisted, and I think that twisted
can use the asyncio event loop if you want.

I don't see how this is related to making the containers more friendly.

pytest vs unittest

Testing with pytest is so much better than anything else and it worth the tiny effort
of doing pip install pytest.

I cannot think about more unrelated topic to making the container more friendly :-)

Let focus on the friendliness issue, because this seem to the purpose of this PR.
Please open an issue about the current containers and explain how the current containers
are used today and what are the issues that students or instructors have with the current
containers.

yaacov · 2023-09-11T16:51:12Z

asyncio does not replace twisted - it is just the core of twisted, and I think that twisted
can use the asyncio event loop if you want.

For basic HTTP client server communication, you don't need twisted, asyncio is doing all that is needed.
Anything fancier that require twisted will make it harder to implement using tools and languages that don't have twisted.

I cannot think about more unrelated topic to making the container more friendly :-)

ok :-)

yaacov · 2023-09-11T18:08:37Z

@nirs hi,

a. reverted ci to use pytest, still need to translate the tests.
b. started splitting the PR, now it's split between metadata commits and code commits, still need to split the code commits further.

- update readme - move pipenv to the docs package - update how to check exercises doc Signed-off-by: yzamir <kobi.zamir@gmail.com>

- update makefile - update ci to use make file - remove travis config - update git ignore - update helper scripts Signed-off-by: yzamir <kobi.zamir@gmail.com>

- remove compiled examples that will only work on one version of python Signed-off-by: yzamir <kobi.zamir@gmail.com>

- add settings page - make the dashboard lighter Signed-off-by: yzamir <kobi.zamir@gmail.com>

- make a rose-common container based on ubi ptyhon - remove classes and methods that are not used (network, configuration) Signed-off-by: yzamir <kobi.zamir@gmail.com>

- move from twisted to asyncio - move from autobahn to websockets - add specific configuration files - add makefile - add dockerfile Signed-off-by: yzamir <kobi.zamir@gmail.com>

Signed-off-by: yzamir <kobi.zamir@gmail.com>

- move from twisted to asyncio - move from autobahn to websockets - move files into a game directory Signed-off-by: yzamir <kobi.zamir@gmail.com>

Signed-off-by: yzamir <kobi.zamir@gmail.com>

bennypowers · 2023-09-12T16:33:39Z

rose/server/web/settings/settings-form.js

+    }
+
+    render() {
+        this.shadowRoot.innerHTML = `


Interesting take. I would have thought to not use shadow DOM in this case and just this.querySelector, but this is good too

You may choose to use a template element is a static init block, but this isn't reusable so the gains are few

If you'll stick with shadow DOM, use a constructible style sheet and adopted stylesheets instead of a link element

@bennypowers that was a nice learning experience :-)
not using template, but separated the template and css from the main js component, WDYT ?

- use module - use web component to render the form Signed-off-by: yzamir <kobi.zamir@gmail.com>

Signed-off-by: yzamir <kobi.zamir@gmail.com>

bennypowers · 2023-09-13T11:12:09Z

rose/server/web/settings/index.html

+</head>
+<body class="dark-theme">
+    <h2 class="heading">Set Game Drivers</h2>
+    <settings-form class="form" />


custom-elements can't be void

Suggested change

<settings-form class="form" />

<settings-form class="form"></settings-form>

wow, fixed, btw/ why did it work ?

I see it was a hot topic 🔥
WICG/webcomponents#624

Signed-off-by: yzamir <kobi.zamir@gmail.com>

yaacov · 2024-01-10T11:20:49Z

Closing in favor of specific repository per game container:

component	repo
Game engine	https://github.com/RedHat-Israel/rose-game-engine
Game web based user interface	https://github.com/RedHat-Israel/rose-game-web-ui
Game car driving module	https://github.com/RedHat-Israel/rose-game-ai

I also added a reference container for running machine learning driver:
https://github.com/RedHat-Israel/rose-game-ai-reference

to run on all components on a cluster:

kubectl apply -f https://raw.githubusercontent.com/RedHat-Israel/rose-game-engine/main/rose-game.yaml

to run locally:

podman run --rm --network host -it quay.io/rose/rose-game-engine:latest
podman run --rm --network host -it quay.io/rose/rose-game-web-ui:latest

and for the driver, create mydriver.py file locally, and run:

podman run --rm --network host -it \
  -v $(pwd)/mydriver.py:/mydriver.py:z \
  -e DRIVER /mydriver.py \
  quay.io/rose/rose-game-ai:latest

bennypowers reviewed Sep 11, 2023

View reviewed changes

yaacov force-pushed the poc-separate-client-server branch 3 times, most recently from 0e6c0a4 to ee55c43 Compare September 11, 2023 15:35

yaacov force-pushed the poc-separate-client-server branch from ee55c43 to d548f5e Compare September 11, 2023 16:36

yaacov force-pushed the poc-separate-client-server branch 2 times, most recently from 22e309d to 1b64380 Compare September 11, 2023 18:03

yaacov force-pushed the poc-separate-client-server branch 2 times, most recently from f64802d to 23670ca Compare September 11, 2023 21:51

bennypowers changed the title ~~Make ROSE containers more freindly~~ Make ROSE containers more friendly Sep 12, 2023

yaacov force-pushed the poc-separate-client-server branch 4 times, most recently from 53afd6b to e2c8463 Compare September 12, 2023 10:36

yaacov added 6 commits September 12, 2023 13:39

Update docs

60f55b6

- update readme - move pipenv to the docs package - update how to check exercises doc Signed-off-by: yzamir <kobi.zamir@gmail.com>

Update CI and gloabl scripts

4b70311

- update makefile - update ci to use make file - remove travis config - update git ignore - update helper scripts Signed-off-by: yzamir <kobi.zamir@gmail.com>

Cleanup examples

a8a9ea7

- remove compiled examples that will only work on one version of python Signed-off-by: yzamir <kobi.zamir@gmail.com>

Update web server public resources

7b73ce5

- add settings page - make the dashboard lighter Signed-off-by: yzamir <kobi.zamir@gmail.com>

Cleanup common package

63f66a8

- make a rose-common container based on ubi ptyhon - remove classes and methods that are not used (network, configuration) Signed-off-by: yzamir <kobi.zamir@gmail.com>

Make client pakage a stand alone application

3daec35

- move from twisted to asyncio - move from autobahn to websockets - add specific configuration files - add makefile - add dockerfile Signed-off-by: yzamir <kobi.zamir@gmail.com>

yaacov added 5 commits September 12, 2023 13:39

Add tests for the updates client application

8272009

Signed-off-by: yzamir <kobi.zamir@gmail.com>

Move public static web resources in the server package

e55caa9

Signed-off-by: yzamir <kobi.zamir@gmail.com>

Move src files of the server application into a source dir

82851ee

- move from twisted to asyncio - move from autobahn to websockets - move files into a game directory Signed-off-by: yzamir <kobi.zamir@gmail.com>

Add a common directory to the server application

f27c553

Signed-off-by: yzamir <kobi.zamir@gmail.com>

Add configuration and CI files to the server application

f3c182c

Signed-off-by: yzamir <kobi.zamir@gmail.com>

yaacov force-pushed the poc-separate-client-server branch from e2c8463 to f3c182c Compare September 12, 2023 10:39

Remove running requirements from the server requirements-dev.txt

327c5b7

Signed-off-by: yzamir <kobi.zamir@gmail.com>

yaacov force-pushed the poc-separate-client-server branch 2 times, most recently from aba7194 to c98bcfb Compare September 12, 2023 12:44

Add rose dir to path when running driver outside a container

530fef5

Signed-off-by: yzamir <kobi.zamir@gmail.com>

yaacov force-pushed the poc-separate-client-server branch 3 times, most recently from 53c301a to e8e4db7 Compare September 12, 2023 16:20

bennypowers reviewed Sep 12, 2023

View reviewed changes

Update settings page

f4b422c

- use module - use web component to render the form Signed-off-by: yzamir <kobi.zamir@gmail.com>

yaacov force-pushed the poc-separate-client-server branch from ece9687 to f4b422c Compare September 12, 2023 18:07

Remove websocket in favor of aiohttp

f911553

Signed-off-by: yzamir <kobi.zamir@gmail.com>

bennypowers requested changes Sep 13, 2023

View reviewed changes

yaacov added 2 commits September 13, 2023 14:23

Split server dockerfile

653d0a7

Signed-off-by: yzamir <kobi.zamir@gmail.com>

web components can't be empty

641e2d0

Signed-off-by: yzamir <kobi.zamir@gmail.com>

yaacov closed this Jan 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make ROSE containers more friendly #485

Make ROSE containers more friendly #485

yaacov commented Sep 11, 2023

yaacov commented Sep 11, 2023

bennypowers Sep 11, 2023

yaacov Sep 12, 2023

yaacov Sep 12, 2023

bennypowers Sep 12, 2023

bennypowers Sep 11, 2023

yaacov Sep 12, 2023

bennypowers Sep 12, 2023

bennypowers Sep 11, 2023

bennypowers Sep 11, 2023

bennypowers Sep 11, 2023

bennypowers Sep 11, 2023

yaacov Sep 12, 2023

yaacov Sep 12, 2023

bennypowers Sep 12, 2023

nirs commented Sep 11, 2023

yaacov commented Sep 11, 2023

yaacov commented Sep 11, 2023

yaacov commented Sep 11, 2023

yaacov commented Sep 11, 2023

yaacov commented Sep 11, 2023

nirs commented Sep 11, 2023

yaacov commented Sep 11, 2023

yaacov commented Sep 11, 2023

bennypowers Sep 12, 2023 •

edited

Loading

yaacov Sep 12, 2023

bennypowers Sep 13, 2023

yaacov Sep 13, 2023

yaacov Sep 13, 2023

yaacov commented Jan 10, 2024

	<button id="reset">Reset</button>
	<button id="reset" type="reset">Reset</button>

	<settings-form class="form" />
	<settings-form class="form"></settings-form>

Make ROSE containers more friendly #485

Make ROSE containers more friendly #485

Conversation

yaacov commented Sep 11, 2023

yaacov commented Sep 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nirs commented Sep 11, 2023

yaacov commented Sep 11, 2023

yaacov commented Sep 11, 2023

yaacov commented Sep 11, 2023

yaacov commented Sep 11, 2023

yaacov commented Sep 11, 2023

nirs commented Sep 11, 2023

yaacov commented Sep 11, 2023

yaacov commented Sep 11, 2023

bennypowers Sep 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yaacov commented Jan 10, 2024

bennypowers Sep 12, 2023 •

edited

Loading